feat(postgres): add sqlx-advisory-locking connection string parameter honored in #[sqlx::test]#4180
feat(postgres): add sqlx-advisory-locking connection string parameter honored in #[sqlx::test]#4180mpecan wants to merge 3 commits intolaunchbadge:mainfrom
sqlx-advisory-locking connection string parameter honored in #[sqlx::test]#4180Conversation
mpecan
force-pushed
the
feat/sqlx-test-locking
branch
from
014bb58 to
3cf761a
Compare
mpecan
force-pushed
the
feat/sqlx-test-locking
branch
from
3cf761a to
501cd1a
Compare
Add a `locking` parameter to `#[sqlx::test]` that controls whether an advisory lock is acquired before creating the test database schema. Defaults to `true` to preserve existing PostgreSQL behavior. Setting `locking = false` allows `#[sqlx::test]` to work with databases that speak the PostgreSQL wire protocol but do not implement advisory locks, such as CockroachDB. When disabled, migrator locking is also skipped so the entire test setup is advisory-lock-free. Follows the same pattern as `Migrator::set_locking()` (PR launchbadge#2063).
mpecan
force-pushed
the
feat/sqlx-test-locking
branch
from
501cd1a to
82f2933
Compare
|
In all likelihood, you would be setting this on all invocations, right? So this probably makes more sense to live in the [macros.test]
locking = falseBut arguably, this is more a property of the database connection itself, so maybe something to put in the |
|
Similar to #4152 |
|
@abonander that is a much better idea, it also enables the code to be DB agnostic, instead having the connection string define behaviour (so we can run the "same" code, from the developers perspective on both DBs). If I understand correctly, would we want to implement the same for the migrator (i.e. Would need a decision on which takes precedence (I would expect calling |
Replace the `#[sqlx::test(locking = false)]` macro attribute with a connection string parameter `?sqlx-advisory-locking=false`. This is a better design because: - Configuration lives where the database identity is declared - Users set it once in DATABASE_URL rather than annotating every test - Follows the existing `statement-cache-capacity` precedent - Named generically to leave the door open for Migrator support When `sqlx-advisory-locking=false` is set, both the test schema setup and migrations skip advisory locks, allowing `#[sqlx::test]` to work with CockroachDB and similar databases.
mpecan
changed the title
sqlx-advisory-locking connection string parameter
|
@abonander implemented the approach you suggested. I don't expect we want to start a CockroachDB integration test, any other suggestions as to how I can improve the test coverage of this case? I tried to do it in a way where I don't break any tests, but I cannot test against PostgreSQL since that is where we do need the advisory locks. |
mpecan
changed the title
sqlx-advisory-locking connection string parametersqlx-advisory-locking connection string parameter honored in #[sqlx::test]
2 participants
Adds a
sqlx-advisory-lockingconnection string parameter toPgConnectOptionsthat controls whether#[sqlx::test]acquires advisory locks during test database setup and migrations.Since #3753 switched test setup to
pg_advisory_xact_lock, databases that speak the PostgreSQL wire protocol but don't implement advisory locks (e.g. CockroachDB, see cockroachdb/cockroach#13546) cannot use#[sqlx::test]at all.Design change
The original version of this PR added a
locking = falseattribute to the#[sqlx::test]macro. Based on maintainer feedback, the approach was redesigned to use a connection string parameter instead. This is a better fit because:DATABASE_URL)statement-cache-capacityprecedent for sqlx-specific URL parametersNamed generically (
sqlx-advisory-locking) rather than test-specific, soMigratorcan respect it in the future if desired — but current scope is test setup only.Usage
Set in
DATABASE_URL— no per-test annotation needed:All
#[sqlx::test]tests automatically respect the setting: both test database schema setup and migrations skip advisory locks.Does your PR solve an issue?
Partially addresses #198 (CockroachDB support).
Is this a breaking change?
No. Default behavior is unchanged (
advisory_locking = true). Advisory locks are only skipped when explicitly opted out via the connection string.